Skip to content

CLN: Add more methods to whitelist (for now) #5514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

jtratner
Copy link
Contributor

No test cases because goal is just to make sure this is not blocked in
0.13. Can do more tweaks later. If I have time I will try to pull
together something to cover what we were discussing in #5480.

@hayd
Copy link
Contributor

hayd commented Nov 14, 2013

"for now" ?

@jtratner
Copy link
Contributor Author

As in, do it for 0.13 and then figure out about adding test cases and
whatever else later on when we have more time to discuss (since both you
and @gdraps had issues with things not being whitelisted that were supposed
to be).

@hayd
Copy link
Contributor

hayd commented Nov 14, 2013

ok, sweet. I wonder if these are low hanging fruit for vector / cythoning.

@jreback
Copy link
Contributor

jreback commented Nov 27, 2013

@jtratner what's the status of thsi?

@gdraps
Copy link
Contributor

gdraps commented Nov 27, 2013

I have a branch with a tentative PR that I can submit later today.

One hitch found while testing is that the whitelist is used for groupby tab
completion but some methods in it are specific to Series or DataFrame
(dtype, corrwith, value_counts). Any thoughts on tailoring the tab
completion based on groupby object type, or should the whitelist be limited
to common methods?
On Nov 27, 2013 10:34 AM, "jreback" [email protected] wrote:

@jtratner https://github.com/jtratner what's the status of thsi?


Reply to this email directly or view it on GitHubhttps://github.com//pull/5514#issuecomment-29408564
.

@jtratner
Copy link
Contributor Author

I'd be inclined to just leave it for both...once you try to use it you'll
learn that it's not available anyways. That way we can keep things more
simple.

@hayd
Copy link
Contributor

hayd commented Nov 27, 2013

The other option is to add it in the Series/DataFrameGroupby class rather than to the whitelist.

@gdraps
Copy link
Contributor

gdraps commented Nov 28, 2013

ok, I put a quick fix PR out there: #5604. I used @jtratner's approach of leaving Series/DataFrame-specific methods in the whitelist (dtype, dtypes, boxplot, corrwith, and value_counts). Feel free to use it, or not.

@jreback
Copy link
Contributor

jreback commented Dec 5, 2013

closing in favor of #5604

@jreback jreback closed this Dec 5, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants